Conversation
|
Notes: Test failures are shared with |
| callerInfo := headers.GetCallerInfo(ctx) | ||
| tags = append(tags, metrics.NexusCallerTag(callerInfo.CallerName)) | ||
| // Namespace is the only valid caller_type. To be updated when there are more kinds | ||
| tags = append(tags, metrics.NexusCallerTypeTag("namespace")) |
There was a problem hiding this comment.
Not sure if there's a better way to do this. callerInfo.CallerType seems to be something else (which is maybe a hint that using callerInfo at all is wrong here?).
There was a problem hiding this comment.
yeah a bit confusing that there is
CallerTypeAPI = "api"
which is a header.
|
Not reviewing this yet since we have all of the metrics in different forms already. Let's see what @kepe-temporal comes back with. |
stephanos
force-pushed
the
feature/nexus-standalone
branch
from
38923fc to
340f40a
Compare
stephanos
force-pushed
the
feature/nexus-standalone
branch
from
7ae0421 to
414a003
Compare
stephanos
force-pushed
the
feature/nexus-standalone
branch
3 times, most recently
from
ce242b7 to
397eca3
Compare
S15
force-pushed
the
feature/nexus-standalone
branch
from
397eca3 to
7ae0421
Compare
stephanos
force-pushed
the
feature/nexus-standalone
branch
2 times, most recently
from
e36b74b to
bcda346
Compare
|
We can use the old metrics and rename them, but we can't split metrics across multiple tables on ingest depending on the value of a label. |
stephanos
force-pushed
the
feature/nexus-standalone
branch
9 times, most recently
from
a44e612 to
44d2c2b
Compare
stephanos
force-pushed
the
feature/nexus-standalone
branch
10 times, most recently
from
378e20c to
80b781d
Compare
S15
force-pushed
the
nexus-request-metrics
branch
from
6075a82 to
1c9aa39
Compare
S15
changed the title
|
|
||
| // Add caller tags from the context. | ||
| callerInfo := headers.GetCallerInfo(ctx) | ||
| tags = append(tags, metrics.NexusCallerTag(callerInfo.CallerName)) |
There was a problem hiding this comment.
are we certain this is doing the right thing? since the test uses only one namespace it seems I'm not sure I can tell.
| WithDescription("The number of Nexus requests for which pre-processing failed."), | ||
| ) | ||
| NexusRequestErrors = NewCounterDef( | ||
| "nexus_request_error_count", |
There was a problem hiding this comment.
Maybe I'm too late, but why the _count suffix? nexus_completion_requests, nexus_completion_request_preprocess_errors, nexus_request_preprocess_errors etc don't have it.
| // Add caller tags from the context. | ||
| callerInfo := headers.GetCallerInfo(ctx) | ||
| tags = append(tags, metrics.NexusCallerTag(callerInfo.CallerName)) | ||
| // Namespace is the only valid caller_type. To be updated when there are more kinds |
There was a problem hiding this comment.
how about?
| // Namespace is the only valid caller_type. To be updated when there are more kinds | |
| // "namespace" is the only valid nexus_caller_type for now. |
| callerInfo := headers.GetCallerInfo(ctx) | ||
| tags = append(tags, metrics.NexusCallerTag(callerInfo.CallerName)) | ||
| // Namespace is the only valid caller_type. To be updated when there are more kinds | ||
| tags = append(tags, metrics.NexusCallerTypeTag("namespace")) |
There was a problem hiding this comment.
yeah a bit confusing that there is
CallerTypeAPI = "api"
which is a header.
4 participants
What changed?
New Metrics:
Counters
nexus_request_error_countNew Labels:
For
nexus_requests,nexus_request_error_countandnexus_latency. Most of these were already set onnexus_requestsandnexus_latencynamespacenexus_endpointnexus_servicenexus_operationnexus_callernexus_caller_typeHow did you test it?